Add tests for molecules accessible list#81
Conversation
…cript.React.Components into main
…cript.React.Components into main
…cript.React.Components into main
…avaScript.React.Components into main
SaidShah
requested review from
brandongregoryscott,
jstevenson and
wintondeshong
Codecov Report
@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 34.88% 38.76% +3.88%
==========================================
Files 51 51
Lines 903 908 +5
Branches 209 206 -3
==========================================
+ Hits 315 352 +37
+ Misses 588 556 -32
Continue to review full report at Codecov.
|
brandongregoryscott
left a comment
brandongregoryscott
left a comment
There was a problem hiding this comment.
Just one question about that invalid element test, I just want to make sure the test is actually doing what we're thinking it does. If you comment out the logic in AccessibleList that validates that the child is a valid element, does that test break?
| ); | ||
|
|
||
| // Assert | ||
| expect(container.innerHTML).not.toContain("null"); |
There was a problem hiding this comment.
I wonder if this is actually testing what we think we're testing. I've seen null used as a "short-circuit" return in a component that is not meant to render due to a certain condition, but that doesn't mean it renders the text "null" in the DOM. Is there a way to easily check for number of children from the returned container? Or perhaps that every child is of type button, since that's the only valid element we're using as children?
There was a problem hiding this comment.
@brandongregoryscott I removed the check for valid react element to test and it does break when passing in null and undefined. I changed the Assert to check and make sure the total child nodes being rendered is a value of 2 since we are only creating 2 valid nodes. I think this would address your concerns, but please let me know if you would like any other changes.
There was a problem hiding this comment.
Thanks for double checking that. Weird, I would not expect null or undefined to be in the inner HTML. Sorry for the late response, this looks good to go 👍
2 participants
Closes #19